chore: apply 51278a70d remediations onto origin/main#851
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughPlaywright reduced-motion settings, UI and audit regression assertions, sitemap expectations, and Supabase drift and schema-test expectations were updated. ChangesRegression contracts
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
CI triageCI failed on this PR. Automated classification of the 3 failed job(s):
Compared with main CI run #3269 (success). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/supabase-schema.test.ts (1)
1374-1399: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftAlign the replay source, schema snapshot, manifest, and assertions.
These updates cannot pass against the checked migration set: CI reports the loaded migration still uses
pg_catalog.to_jsonb, retains the legacy query corrector, and ends with20260717173000_reassert_supabase_admin_default_privileges.sql. The manifest also removes title-word checks thatsupabase/schema.sqlstill creates.
tests/supabase-schema.test.ts#L1374-L1399: apply the intended SQL changes to the replayed migration, or make the assertions match the semantically valid current SQL.tests/supabase-schema.test.ts#L1222-L1222: expect the actual final migration (20260717173000_reassert_supabase_admin_default_privileges.sql) unless that migration is intentionally removed.supabase/drift-manifest.json#L7576-L7580: update the migration/schema snapshot for the removed constraints, then regenerate the manifest.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/supabase-schema.test.ts` around lines 1374 - 1399, Align the replayed migration, schema snapshot, manifest, and assertions with the current migration set: in tests/supabase-schema.test.ts lines 1374-1399, apply the intended SQL changes or assert the semantically valid current SQL; at line 1222, expect 20260717173000_reassert_supabase_admin_default_privileges.sql unless it is intentionally removed; and in supabase/drift-manifest.json lines 7576-7580, update the snapshot for the removed constraints and regenerate the manifest.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/audit-content-services-regressions.test.ts`:
- Around line 209-217: Align the source-link assertion in the test with the
implementation in the form-detail component: either restore the conditional
structure expected by the normalized source check, or update the assertions to
match the component’s stable rendered markup. Ensure the test passes while
preserving the single available-link expectation and rejecting “Source link
pending.”
In `@tests/audit-navigation-auth-regressions.test.ts`:
- Around line 126-128: Reconcile the H1 expectation in the test with the markup
produced by ClinicalDashboard: either update ClinicalDashboard’s root heading to
provide the intended accessible “Clinical KB” H1 contract or revise the
assertion to match its stable rendered structure. Ensure exactly one root H1
remains and the accessible title is preserved.
In `@tests/site-map.test.ts`:
- Around line 125-130: Regenerate the checked-in sitemap artifact using the
repository’s existing sitemap generator so the public route handlers, including
/applications, appear in the Main product routes section and satisfy the
assertions in expectedProductHandlers. Commit the generated output without
changing the test expectations.
---
Outside diff comments:
In `@tests/supabase-schema.test.ts`:
- Around line 1374-1399: Align the replayed migration, schema snapshot,
manifest, and assertions with the current migration set: in
tests/supabase-schema.test.ts lines 1374-1399, apply the intended SQL changes or
assert the semantically valid current SQL; at line 1222, expect
20260717173000_reassert_supabase_admin_default_privileges.sql unless it is
intentionally removed; and in supabase/drift-manifest.json lines 7576-7580,
update the snapshot for the removed constraints and regenerate the manifest.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 434e47fb-cbfb-4279-b506-f496236d8e2e
📒 Files selected for processing (9)
playwright.config.tssupabase/drift-manifest.jsontests/audit-content-services-regressions.test.tstests/audit-navigation-auth-regressions.test.tstests/site-map.test.tstests/supabase-schema.test.tstests/ui-accessibility.spec.tstests/ui-smoke.spec.tstests/ui-tools.spec.ts
| expect(normalizedFormDetailSource).toContain( | ||
| '{form.source?.url ? "Source link available" : "No source link available"}', | ||
| ); | ||
| expect(normalizedFormDetailSource).toMatch(/\{form\.source\?\.url \? \( <a href=\{form\.source\.url\}/); | ||
| expect(normalizedFormDetailSource).toMatch( | ||
| /<a href=\{form\.source\.url\} target="_blank" rel="noopener noreferrer" className="inline-flex min-h-10/, | ||
| /<a href=\{form\.source\.url\} target="_blank" rel="noopener noreferrer" className="inline-flex min-h-tap/, | ||
| ); | ||
| expect(formDetailSource.match(/Source link pending/g)).toHaveLength(1); | ||
| expect(formDetailSource).toContain("Source link pending"); | ||
| expect(formDetailSource.match(/Source link available/g)).toHaveLength(1); | ||
| expect(formDetailSource).not.toContain("Source link pending"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Align this source-link contract with the component.
The reported unit test fails because form-detail-page.tsx does not contain the exact conditional required here. Restore the intended component contract or update this assertion to the component’s stable rendered structure; the current PR leaves CI failing.
🧰 Tools
🪛 GitHub Check: Unit coverage
[failure] 209-209: [node] tests/audit-content-services-regressions.test.ts > content and services audit regressions > claims and renders a form source link only when the record has a URL
AssertionError: expected '"use client"; import { useRouter } fr…' to contain '{form.source?.url ? "Source link avai…'
- Expected
- Received
- {form.source?.url ? "Source link available" : "No source link available"}
- "use client"; import { useRouter } from "next/navigation"; import { ArrowLeft, Bookmark, BookmarkCheck, CalendarDays, CircleCheck, ChevronRight, Clipboard, ClipboardList, Clock3, Download, ExternalLink, FileText, Info, MapPin, Navigation, Phone, Route, Scale, ShieldCheck, Tag, UserRound, X, CircleX, type LucideIcon, } from "lucide-react"; import { useMemo, useState, type ReactNode } from "react"; import { cn, codeText, floatingControl, metadataPill, pageContainer, primaryControl, textMuted, toneDanger, toneInfo, toneNeutral, toneSuccess, toneWarning, } from "
@/components/ui-primitives"; import { FormCodeBadge, splitFormCode } from "@/components/forms/form-code-badge"; import { appModeHomeHref } from "@/lib/app-modes"; import { formCatalogDetails, type FormRecord } from "@/lib/form-ranker"; import type { ServiceChipTone, ServiceContact, ServiceCriterion, ServiceSummaryCard } from "@/lib/service-ranker"; import { readSavedRegistrySlugs, savedFormsStorageKey, writeSavedRegistrySlugs } from "@/lib/saved-registry-storage"; const missingText = "Not listed"; function hasText(value: string | null | undefined): value is string { return Boolean(value && value.trim().length > 0); } function displayText(value: string | null | undefined, fallback = missingText) { return hasText(value) ? value.trim() : fallback; } async function copyText(value: string) { if (navigator.clipboard?.writeText) { try { await navigator.clipboard.writeText(value); return; } catch { // Fall through to the legacy selection path for restricted browser contexts. } } const textArea = document.createElement("textarea"); textArea.value = value; textArea.setAttribute("readonly", ""); textArea.style.position = "fixed"; textArea.style.opacity = "0"; document.body.appendChild(textArea); textArea.select(); try { const copied = document.execCommand?.("copy"); if (copied === false) throw new Error("copy command rejected"); } finally { document.body.removeChild(textArea); } } function chipToneClass(tone: ServiceChipTone | null | undefined) { if (tone === "danger") return toneDanger; if (tone === "info") return toneInfo; if (tone === "warning") return toneWarning; if (tone === "success") return toneSuccess; return toneNeutral; } export function sourceToneClass(form: FormRecord) { const status = form.source?.status?.toLowerCase() ?? ""; if (/required|review|unverified|not verified|unchecked|pending|unknown|confirm/.test(status)) return toneWarning; if (form.verification?.locallyVerified === true) return toneSuccess; return toneNeutral; } function formCode(form: FormRecord) { const details = formCatalogDetails(form); if (details?.form) return details.form; if (form.slug.includes("transport")) return "4A"; if (form.slug.includes("capacity")) return "CAP"; if (form.slug.includes("clozapine")) return "CLZ"; if (form.slug.includes("handover")) return "SAFE"; return form.title .split(/\s+/) .map((part) => part[0]) .join("") .slice(0, 4) .toUpperCase(); } function formShortTitle(form: FormRecord) { const details = formCatalogDetails(form); return details?.form ?Form ${details.form}: displayText(form.catalogueLabel, "Form"); } function summaryIcon(card: ServiceSummaryCard) { const label =${card.id} ${card.label} ${card.title}.toLowerCase(); const Icon = label.includes("clock") ? Clock3 : label.includes("destination") || label.includes("place") || label.includes("route") ? MapPin : label.includes("authority") || label.includes("maker") ? UserRound : label.includes("criteria") || label.includes("threshold") ? Scale : ClipboardList; return ; } function summaryCardsFor(form: FormRecord): ServiceSummaryCard[] { if (form.summaryCards?.length) return form.summaryCards.slice(0, 4); return [ { id: "route", label: "Route", title: "Use pathway", detail: form.route }, { id: "eligibility", label: "Eligibility", title: "Patient fi
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/audit-content-services-regressions.test.ts` around lines 209 - 217,
Align the source-link assertion in the test with the implementation in the
form-detail component: either restore the conditional structure expected by the
normalized source check, or update the assertions to match the component’s
stable rendered markup. Ensure the test passes while preserving the single
available-link expectation and rejecting “Source link pending.”
Source: Linters/SAST tools
| it("keeps the root dashboard H1 as Clinical KB", () => { | ||
| expect(clinicalDashboardSource.match(/<h1\b/g)).toHaveLength(1); | ||
| expect(clinicalDashboardSource).toMatch(/<h1 className="sr-only">\s*Clinical Guide\s*<\/h1>/); | ||
| expect(clinicalDashboardSource).toMatch(/<h1 className="sr-only">\s*Clinical KB\s*<\/h1>/); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reconcile the H1 assertion with ClinicalDashboard.
This test currently fails because the dashboard source does not match the exact className="sr-only" H1 markup. Update the component to the intended accessibility contract or revise the assertion to the actual stable structure before merging.
🧰 Tools
🪛 GitHub Check: Unit coverage
[failure] 128-128: [node] tests/audit-navigation-auth-regressions.test.ts > audit navigation and auth regressions > keeps the root dashboard H1 as Clinical KB
AssertionError: expected '"use client";\n\nimport { useRouter, …' to match /
\s*Clinica…/h1>
- Expected:
/\sClinical KB\s</h1>/
- Received:
""use client";
import { useRouter, useSearchParams } from "next/navigation";
import dynamic from "next/dynamic";
import {
CircleAlert,
BookOpen,
ChevronDown,
Clock3,
ExternalLink,
FileImage,
FileText,
FolderOpen,
Heart,
ListChecks,
Loader2,
Quote,
RefreshCw,
Search,
ShieldAlert,
Square,
UploadCloud,
WifiOff,
Wrench,
} from "lucide-react";
import { type CSSProperties, useCallback, useEffect, useMemo, useReducer, useRef, useState } from "react";
import { type DocumentDeleteResult } from "@/components/DocumentManagementActions";
import { extractSafetyFindings } from "@/lib/clinical-safety";
import { isLocalNoAuthMode, publicUploadsEnabled } from "@/lib/client-env";
import { readLocalProjectIdentity, unsafeLocalProjectMessage } from "@/lib/local-project-identity";
import { isDeployedClinicalKb } from "@/lib/deployed-app";
import {
appBackdrop,
answerSurface,
cn,
EmptyState,
floatingControl,
InlineNotice,
primaryControl,
textMuted,
} from "@/components/ui-primitives";
import { useAuthSession } from "@/lib/supabase/client";
import { AccountSetupDialog } from "@/components/clinical-dashboard/account-setup-dialog";
import { CrossModeLinksSection } from "@/components/clinical-dashboard/cross-mode-links";
import { useEventCallback } from "@/components/clinical-dashboard/use-event-callback";
import { AuthPanel } from "@/components/clinical-dashboard/auth-panel";
import { buildMobileSectionFabState, MobileSectionFab, ToolsHub } from "@/components/clinical-dashboard/dashboard-nav";
import { SettingsDialog } from "@/components/clinical-dashboard/settings-dialog";
import { useSidebarCollapsed } from "@/components/clinical-dashboard/use-sidebar-collapsed";
import { useTheme } from "@/components/clinical-dashboard/use-theme";
import {
deriveSidebarIdentity,
ClinicalDesktopSidebar,
ClinicalMobileSidebar,
} from "@/components/clinical-dashboard/ClinicalSidebar";
import {
SetupChecklist,
UploadPanel,
IndexingMonitor,
IngestionQualityConsole,
LibraryHealthStrip,
fallbackSetupChecks,
hasReadyRequiredPublicSearchConfig,
hasReadyPublicSearchSetup,
type SetupCheck,
type IngestionQualityReviewItem,
} from "@/components/clinical-dashboard/DocumentManagerPanel";
import { GuideDialog, GuideTrigger, UtilityDrawer } from "@/components/clinical-dashboard/dashboard-shell";
import { sanitizeAnswerDisplayText, sanitizeDisplayText } from "@/components/clinical-dashboard/display-text";
import {
isPreformattedGroundedAnswer,
NaturalLanguageAnswer,
ScopeAndGovernanceNotice,
UserQuestionBubble,
} from "@/components/clinical-dashboard/answer-content";
import { AnswerEmptyState, AnswerProgressStepper, AnswerSkeleton } from "@/components/clinical-dashboard/answer-status";
import {
type AnswerProgressUpdate,
type TimedAnswerProgressUpdate,
} from "@/components/clinical-dashboard/answer-progress";
import { evidenceMapRowsFromRenderModel } from "@/components/clinical-dashboard/evidence-map-model";
import { MasterSearchHeader } from "@/components/clinical-dashboard/master-search-header";
import { UniversalSearchAlsoMatches } from "@/components/clinical-dashboard/universal-search-also-matches";
import { useScrollHideReporter } from "@/components/clinical-dashboard/use-hide-on-scroll";
import { SearchCommandProvider } from "@/components/clinical-dashboard/search-command-context";
import {
answerReferencesDocument,
answerTimedOutError,
applyRenamedDocumentToAnswer,
compactScopeFilters,
hasActiveIndexingWork,
hasNonProductionSupabaseApiKeyFallback,
isAbortError,
mergeDocumentRefresh,
normalizeNavigationHash,
setupNeedsSlowRecheck,
setupRecheckPollMs,
shorterPollDelay,
} from "@/components/clinical-dashboard/clinical-dashboard-helpers";
import { answerRecovery, errorCopy } from "@/lib/ui-copy";
imp
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/audit-navigation-auth-regressions.test.ts` around lines 126 - 128,
Reconcile the H1 expectation in the test with the markup produced by
ClinicalDashboard: either update ClinicalDashboard’s root heading to provide the
intended accessible “Clinical KB” H1 contract or revise the assertion to match
its stable rendered structure. Ensure exactly one root H1 remains and the
accessible title is preserved.
Source: Linters/SAST tools
| for (const [route, file, target] of expectedProductHandlers) { | ||
| expect(data.publicRouteHandlers).toContainEqual({ route, file }); | ||
| expect(data.apiRoutes).not.toContainEqual({ route, file }); | ||
| expect(data.redirects).toContainEqual({ route, file, target }); | ||
| expect(productSection).toContain(`\`${route}\``); | ||
| expect(apiSection).not.toContain(`\`${route}\``); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Regenerate the checked-in sitemap artifact.
The test now requires these public handlers in ## Main product routes, but the reported sitemap contents omit /applications, so Line 129 fails. Regenerate and commit the sitemap from the repository’s generator (or intentionally revise the generation contract); do not leave this assertion red.
🧰 Tools
🪛 GitHub Check: Unit coverage
[failure] 129-129: [node] tests/site-map.test.ts > tracked sitemap > keeps public redirect handlers in product routes and API handlers in the API section
AssertionError: expected '## Main product routes\n\n- / - Mai…' to contain '/applications'
- Expected
- Received
/applications
-
Main product routes
-
/- Main Clinical KB shell. Source:src/app/page.tsx.
-
/differentials- Differentials home and search surface. Source:src/app/differentials/page.tsx.
-
/differentials/diagnoses- Diagnosis stream. Source:src/app/differentials/diagnoses/page.tsx.
-
/differentials/presentations/[slug]- Route discovered from app directory Source:src/app/differentials/presentations/[slug]/page.tsx.
-
/documents/search- Documents search command centre. Source:src/app/documents/search/page.tsx.
-
/documents/source- Compatibility redirect to the canonical live document viewer when a valid id is supplied. Source:src/app/documents/source/page.tsx.
-
/documents/source/evidence- Compatibility redirect sharing the canonical live document viewer handoff. Source:src/app/documents/source/evidence/page.tsx.
-
/dsm- DSM-5 Diagnosis home. Source:src/app/dsm/page.tsx.
-
/dsm/compare- DSM diagnosis comparison. Source:src/app/dsm/compare/page.tsx.
-
/dsm/search- DSM diagnosis search and catalogue browser. Source:src/app/dsm/search/page.tsx.
-
/factsheets- Route discovered from app directory Source:src/app/factsheets/page.tsx.
-
/factsheets/[slug]- Route discovered from app directory Source:src/app/factsheets/[slug]/page.tsx.
-
/factsheets/search- Route discovered from app directory Source:src/app/factsheets/search/page.tsx.
-
/favourites- Saved clinical items and sets. Source:src/app/favourites/page.tsx.
-
/forms- Forms home and search surface. Source:src/app/forms/page.tsx.
-
/formulation- Clinical formulation home and local mechanism search surface. Source:src/app/formulation/page.tsx.
-
/formulation/builder- Structured clinical formulation builder. Source:src/app/formulation/builder/page.tsx.
-
/formulation/compare- Side-by-side mechanism comparison. Source:src/app/formulation/compare/page.tsx.
-
/formulation/map- Formulation mechanism domain map. Source:src/app/formulation/map/page.tsx.
-
/privacy- Privacy and data-processing governance draft. Source:src/app/privacy/page.tsx.
-
/reference/colour-coding- Route discovered from app directory Source:src/app/reference/colour-coding/page.tsx.
-
/services- Services home and search surface. Source:src/app/services/page.tsx.
-
/specifiers- Psychiatric specifier home and local search surface. Source:src/app/specifiers/page.tsx.
-
/specifiers/builder- Structured diagnostic wording builder. Source:src/app/specifiers/builder/page.tsx.
-
/specifiers/compare- Side-by-side psychiatric specifier comparison. Source:src/app/specifiers/compare/page.tsx.
-
/specifiers/map- Psychiatric specifier family map. Source:src/app/specifiers/map/page.tsx.
-
/therapy-compass- Therapy Compass home (source-grounded therapy decision support). Source:src/app/therapy-compass/page.tsx.
-
/therapy-compass/[slug]/brief- Therapy brief-intervention view. Source:src/app/therapy-compass/[slug]/brief/page.tsx.
-
/therapy-compass/[slug]/sheet- Therapy patient-sheet builder. Source:src/app/therapy-compass/[slug]/sheet/page.tsx.
-
/therapy-compass/compare- Side-by-side therapy comparison. Source:src/app/therapy-compass/compare/page.tsx.
-
/therapy-compass/pathways- Problem-based clinical therapy pathways. Source:src/app/therapy-compass/pathways/page.tsx.
-
/therapy-compass/recommend- Recommend a therapy from a clinical question and constraints. Source:src/app/therapy-compass/recommend/page.tsx.
-
/therapy-compass/review- Therapy records awaiting qualified-clinician source review. Source:src/app/therapy-compass/review/page.tsx.
-
/therapy-compass/search- Therapy library search surface. Source:src/app/therapy-compass/search/page.tsx.
-
/tools- Route discovered from app directory Source:src/app/tools/page.tsx.
�
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/site-map.test.ts` around lines 125 - 130, Regenerate the checked-in
sitemap artifact using the repository’s existing sitemap generator so the public
route handlers, including /applications, appear in the Main product routes
section and satisfy the assertions in expectedProductHandlers. Commit the
generated output without changing the test expectations.
Source: Linters/SAST tools
Summary
51278a70dremediation set ontoorigin/mainwhile preserving currentmainbehavior.d8739ed02) with no runtime code changes beyond the reviewed remediation scope.Verification
npm run verify:pr-localduring local iteration (initially blocked by missingprettier/toolchain modules in this worktree; rerun pending full toolchain install).npm run verify:cheap(runtime portion passed; final gate failed at tooling stage:scripts/generate-site-map.tscould not resolveprettierin this checkout).npm run verify:uiwhen UI, routing, styling, browser behavior, reduced-motion, or forced-colors behavior changedplaywright.config.ts) blocked production/advisory UI lanes; hosted UI checks were re-run after this fix to refresh before merge.npm run verify:releasebefore release or handoff confidence claimsFor retrieval, ranking, selection, chunking, source/citation rendering, or answer-contract changes,
verify:pr-localrunseval:rag:offlineautomatically.npm run check:production-readinesswhen clinical workflow, privacy, Supabase, source governance, or deployment behavior changednpm run check:deployment-readinesswhen deployment startup, hosting, or rollout behavior changedRisk and rollout
d8739ed02and re-rungh pr checkoutplus smoke checks; no data migrations or schema changes shipped.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes
prettier) in this worktree; all required checks will be revalidated by hosted CI after fix.Summary by CodeRabbit
Bug Fixes
Tests